Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authorization header issue before initialize SwaggerUI #73

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mur-wtag
Copy link

@mur-wtag mur-wtag commented Sep 28, 2016

CORE-826

Bug Description:

Consider this code block:

This code block going to initialize SwaggerUi.
But To initialize SwaggerUi we need to have authorization header. And the inside onComplete we are adding Authorization Header by calling addApiKeyAuthorization().

So, initialization of SwaggerUi fails and triggers onFaiulre :p
And this resolution is to add header before initialize SwaggerUI

Some discussions:

@grape-bot
Copy link

grape-bot commented Sep 28, 2016

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.
1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Here's an example of a CHANGELOG.md entry:

* [#73](https://github.com/ruby-grape/grape-swagger-rails/pull/73): Authorization header issue before initialize `SwaggerUI` - [@mur-wtag](https://github.com/mur-wtag).

Generated by 🚫 danger

@dblock
Copy link
Member

dblock commented Sep 28, 2016

Do you think you can write a test that actually reproduces this?

@serggl
Copy link
Member

serggl commented Oct 3, 2016

@mur-wtag I think the idea was to create a capybara/selenium spec that with will fail unless your fix is available. See https://github.com/ruby-grape/grape-swagger-rails/blob/master/spec/features/swagger_spec.rb to find spec samples

@dblock
Copy link
Member

dblock commented Oct 3, 2016

+1 @serggl, right now these specs pass, so make a spec that fails, apply a fix and see it succeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants